Fix relayStylePagination to better support existing and incoming without edges property #9201
Conversation
|
@alexstrat: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
…-7944-relay-style-pagination-add-empty-edges-array-while-we-dont-know
alexstrat
changed the title
…ion-add-empty-edges-array-while-we-dont-know
There was a problem hiding this comment.
Thanks for digging into this @alexstrat! A couple initial questions below to steer your thinking.
If you run out of time to work on this, just let me know and I/we (the AC team) can take over.
src/utilities/policies/pagination.ts
Outdated
| let merged: SafeReadonly<TExistingRelay<TNode, TConnectionExtraData>> = { | ||
| ...(existing || {}), | ||
| ...incoming, | ||
| } | ||
|
|
||
| if (!incoming.edges) { | ||
| return merged |
There was a problem hiding this comment.
If I'm reading this correctly, the entire existing.pageInfo object will be replaced with incoming.pageInfo (not merged) whenever !incoming.edges?
I agree we should do something defensive in this case, but I don't think this version of merged is what we want to return.
There was a problem hiding this comment.
Having !incoming.edges and incoming.pageInfo means that the query that produced this incoming does not select edges field but does select pageInfo, right?
Then, actually, I can't figure out a case where one would make a query that selects pageInfo and not edges, so I have some difficulty to come up with something logic.
What would you recommend doing?
benjamn
added
2021-12
🏓 awaiting-contributor-response
…ion-add-empty-edges-array-while-we-dont-know
|
@benjamn thanks for reading and your questions! Sorry for the late response, I took some time off during the holidays! Please, continue your review, I'll be able to work on it. |
|
@benjamn just a reminder that this PR is still apriori ready for a review - in opposite to the tag 🏓 awaiting-contributor-response seems to specify. |
benjamn
removed
the
🏓 awaiting-contributor-response
|
@benjamn , I'm also running into this. What is needed to get this looked at? |
This PR is suggested fix for #7944
The main problem comes from the fact the current implementation assumes that
edges(andpageInfo) will always be present in non-null incoming and existing, and forces an empty edges array in cache. This PR debunks this assumption and add some tests of the expected behavior in these cases.Checklist: